- 
                Notifications
    You must be signed in to change notification settings 
- Fork 1
TD-6277 remove nhse tel frontend and replace with nhsuk frontend #3392
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
TD-6277 remove nhse tel frontend and replace with nhsuk frontend #3392
Conversation
Alternative: Did not delete and regenerate files.
…s and meta tag change for mobiles see https://service-manual.nhs.uk/design-system/guides/updating-to-v10
…ing older browsers so removing polyfill reference and not replacing it
Comment: May be desireable to consider a task to pass the nonce in header, base controller in viewbag and randomly generate it
| </ItemGroup> | ||
|  | ||
| <ItemGroup> | ||
| <None Include="node_modules\nhsuk-frontend\dist\nhsuk-9.1.0.min.js" /> | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this may not be right despite the docs. Or it may be because the files are created on publish only, however i did manually move the files to wwwroot from node.
…-NHSE-TEL-FRONTEND-and-replace-with-NHSUK-FRONTEND
| There were yarn lock clashes i went with incoming because i hadnt particularly deliberately changed these i dont think | 
| Also see https://hee-tis.atlassian.net/browse/TD-6302 created because of cancellink view component not compatible with 10.x nhse-frontend yet | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please see the inline comments.
Also, there are some image changes in the wwwroot folder (towards the end), are these changes intended? Generally we keep this folder in gitignore. Could you confirm this, please?
| import { JSDOM } from 'jsdom'; | ||
| import { exitFullscreen, enterFullscreen } from '../../learningMenu/fullscreen'; | ||
|  | ||
| /*qqqq what is this page do i need to update it*/ | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you remove comments like these, please?
| ); | ||
|  | ||
| migrationRunner.MigrateUp(); | ||
| migrationRunner.MigrateUp(); | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file has only whitespace change. We understand that you might have used the formatter that has created these, but please could we undo them, please?
|  | ||
| <button class="nhsuk-button delete-button" type="submit">Remove framework source</button> | ||
| </form> | ||
| <div class="nhsuk-link"> | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Like in this file, there's just this 1 change in this line but Github highlights the whole file since there are whitespace changes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah i dont know why its done that okay
Like in this file, there's just this 1 change in this line but Github highlights the whole file since there are whitespace changes.
| "UnitTestConnection": "Data Source=localhost;Initial Catalog=mbdbx101_uar_test;Integrated Security=True;TrustServerCertificate=true;" | ||
| }, | ||
| "CurrentSystemBaseUrl": "https://localhost:5001", | ||
| "CurrentSystemBaseUrl": "https://localhost:5001", | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
another example of whitespace change
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You have refrenced TD-6277 in the PR, that ticket belongs to the moodle area by the description of it. Could you please site the correct ticket number.
| 
 it is because as far as i can tell we dont get the images from nshukfrontend put in by a pipeline. I think it was a week a bit ago. so when updating the package it didnt have images it needs because it now wants them in different paths. So i think i needed to create the folders and put the images in rather than relying on gulp or something like that. So they would need to be source controlled. Unless we are running something to generate them on build, then we can put them a gitignore. I will check first when i update the pull request though because im not certain. | 
JIRA link
TD-6277
Description
Remove TEL-Frontend and replacing with NHSE-Frontend. Then upgrading to NHSE-Frontend 10.x
I have look through a few screen and compared with the NHSE-Frontend showcase, and DLS Production.
The secondary button has changed for example.
The page template has changed. And svgs have been dropped and replaced with pseudos with css shapes.
Because of this using nhsuk-back no longer works with anything but the cancel buttons, it had been used for other purposes like cancel. Instead the change decided on was to use nhsuk-link and remove the svg. So cancel should no longer have an X or a chevron for example.
The View Components package still expects the older version of NHSE-Frontend so will show both the cross and chevron until it has been updated.
There is an extensive list of changes here https://service-manual.nhs.uk/design-system/guides/updating-to-v10
Screenshots
Paste screenshots for all views created or changed: mobile, tablet and desktop, wave analyser showing no errors.
Developer checks
(Leave tasks unticked if they haven't been appropriate for your ticket.)
I have:
Web tests all work the other didnt all work locally for me before the changes, I would like to to be shown how to do this
Either:
Or: